Skip to content

Use cursor copies for PiecewiseSequence slices#8832

Merged
robert3005 merged 8 commits into
developfrom
codex/piecewise-sequence-cursor-copy
Jul 21, 2026
Merged

Use cursor copies for PiecewiseSequence slices#8832
robert3005 merged 8 commits into
developfrom
codex/piecewise-sequence-cursor-copy

Conversation

@danking

@danking danking commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a shared checked cursor-copy helper for PiecewiseSequence slice gathering
  • replace extend_from_slice in the primitive, decimal, VarBinView, and VarBin PiecewiseSequence consumers
  • keep the safety checks explicit: each copy verifies the destination cursor range against the declared output length and capacity, and callers verify the final cursor before set_len

Validation

  • cargo +nightly fmt --all
  • cargo clippy -p vortex-array --all-targets --all-features -- -D warnings
  • cargo bench -p vortex-array --bench listview_rebuild
  • cargo bench -p vortex-array --bench cast_decimal
  • cargo test -p vortex-array piecewise_sequence

@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will improve performance by 25.64%

⚡ 2 improved benchmarks
✅ 1794 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation copy_nullable[65536] 1.4 ms 1 ms +31.84%
⚡ Simulation copy_non_nullable[65536] 1,094.3 µs 913.9 µs +19.73%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

Comparing codex/piecewise-sequence-cursor-copy (542e902) with develop (ba4acb6)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@danking
danking force-pushed the codex/piecewise-sequential-run-take-consumers branch from 9adf56d to c4a0358 Compare July 17, 2026 19:40
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from f9a3472 to afcc04a Compare July 17, 2026 19:41
@danking
danking force-pushed the codex/piecewise-sequential-run-take-consumers branch from c4a0358 to c76531d Compare July 17, 2026 20:05
@danking danking added the changelog/performance A performance improvement label Jul 17, 2026
@danking
danking force-pushed the codex/piecewise-sequential-run-take-consumers branch from 80f7b89 to 30870aa Compare July 17, 2026 20:19
@danking danking mentioned this pull request Jul 17, 2026
Merged
@danking
danking force-pushed the codex/piecewise-sequential-run-take-consumers branch from 30870aa to e777ab4 Compare July 17, 2026 20:25
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from afcc04a to 1a17030 Compare July 17, 2026 20:29
@danking
danking changed the base branch from codex/piecewise-sequential-run-take-consumers to codex/piecewise-sequence-varbin-take July 17, 2026 20:29
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 5924fe3 to 79d9e10 Compare July 17, 2026 20:38
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 1a17030 to 534b70f Compare July 17, 2026 20:39
robert3005
robert3005 reviewed Jul 17, 2026
View reviewed changes
Comment thread vortex-array/src/arrays/decimal/compute/take.rs Outdated
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 79d9e10 to 39ee98e Compare July 20, 2026 16:11
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch 2 times, most recently from 3be0c74 to e60ec21 Compare July 20, 2026 16:37
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch 2 times, most recently from 83c466d to df091fb Compare July 20, 2026 16:56
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch 2 times, most recently from 423876b to 7034fd3 Compare July 20, 2026 18:10
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch 2 times, most recently from e31db3e to 9f02349 Compare July 20, 2026 19:24
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 7034fd3 to bdfe8f7 Compare July 20, 2026 19:24
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 9f02349 to 43ecd40 Compare July 20, 2026 19:42
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from bdfe8f7 to 9c21b5e Compare July 20, 2026 19:42
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 43ecd40 to 97a2e5a Compare July 20, 2026 19:57
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 9c21b5e to b2069a5 Compare July 20, 2026 19:58
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 97a2e5a to 4626cfc Compare July 20, 2026 20:09
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 497ff39 to 5ca0608 Compare July 20, 2026 20:09
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 4626cfc to 9eba049 Compare July 20, 2026 20:13
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 5ca0608 to bfd4825 Compare July 20, 2026 20:13
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 9eba049 to 3c5e519 Compare July 20, 2026 20:16
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from bfd4825 to 1af09ed Compare July 20, 2026 20:17
robert3005
robert3005 reviewed Jul 20, 2026
View reviewed changes
Comment thread vortex-array/src/arrays/piecewise_sequence/spare_buffer_writer.rs Outdated
robert3005
robert3005 reviewed Jul 20, 2026
View reviewed changes
Comment thread vortex-buffer/src/spare_buffer_writer.rs Outdated
@danking
danking force-pushed the codex/piecewise-sequence-varbin-take branch from 3c5e519 to 5d31540 Compare July 20, 2026 22:04
@danking
danking force-pushed the codex/piecewise-sequence-cursor-copy branch from 9183265 to 182567a Compare July 20, 2026 22:04
Base automatically changed from codex/piecewise-sequence-varbin-take to develop July 21, 2026 09:24
danking and others added 5 commits July 21, 2026 10:29
@danking @robert3005
Use cursor copies for PiecewiseSequence slices
6b75bba 
Signed-off-by: Daniel King <dan@spiraldb.com>
@danking @robert3005
Advance PiecewiseSequence copy destinations
c8b7810 
Signed-off-by: Daniel King <dan@spiraldb.com>
@danking @robert3005
Move spare buffer writer to vortex-buffer
9443f05 
Signed-off-by: Daniel King <dan@spiraldb.com>
@claude @robert3005
Make SpareBufferWriter bounds-checked and drop caller-side unsafe
4395d38 
Replace the raw-pointer cursor with a written-count cursor over
spare_capacity_mut, and remove copy_slice_unchecked along with the
unsafe blocks at the four PiecewiseSequence call sites. The bounds
check is one well-predicted branch per slice: benchmarking the gather
loop over 16M u64 elements shows checked copies within noise of
unchecked ones at every run length (212ms vs 200ms even at the
pathological run length of 1, where both beat extend_from_slice's
299ms), so the unchecked variant's extra unsafe surface isn't paying
for anything.

The writer is now #[must_use], and the remaining unsafety is contained
to the copy into the bounds-checked spare-capacity range (the
not-yet-stable <[MaybeUninit<T>]>::write_copy_of_slice) and the single
set_len in finish, justified by the writer's contiguous-prefix
invariant.

Signed-off-by: "Robert" <robert@spiraldb.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GUSY34eRKq9du5zQK1FuZq
@claude @robert3005
Move SpareBufferWriter back into vortex-array
8235e03 
The writer only exists to serve the PiecewiseSequence gather loops, so
keep it pub(crate) in arrays::piecewise_sequence as its own submodule
rather than growing vortex-buffer's public API. It only uses BufferMut's
public surface, so nothing in vortex-buffer changes behavior.

Signed-off-by: "Robert" <robert@spiraldb.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GUSY34eRKq9du5zQK1FuZq
@robert3005
robert3005 force-pushed the codex/piecewise-sequence-cursor-copy branch from 87f6283 to 8235e03 Compare July 21, 2026 09:30
@github-actions

github-actions Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done 542e902 1 Explore Profiling Data
Previous Runs (2)
Status Commit Job Attempt Link
🟢 Done 05ce408 1 Explore Profiling Data
🟢 Done 8235e03 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

github-actions Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries 📖

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +3.7%
Engines: DataFusion No clear signal (+8.9%, medium confidence) · DuckDB No clear signal (-1.3%, low confidence)
Vortex (geomean): 1.001x ➖
Parquet (geomean): 0.983x ➖
Shifts: Parquet (control) -1.7% · Median polish -0.5%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.
datafusion / vortex-file-compressed (1.050x ➖, 0↑ 0↓)
name PR 542e902 (ns) base c42934b (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 9612940 9383131 1.02
vortex_q01/datafusion:vortex-file-compressed 6362699 5912789 1.08
datafusion / parquet (0.964x ➖, 0↑ 0↓)
name PR 542e902 (ns) base c42934b (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 19383185 20234388 0.96
vortex_q01/datafusion:parquet 4512742 4650703 0.97
duckdb / vortex-file-compressed (0.988x ➖, 0↑ 0↓)
name PR 542e902 (ns) base c42934b (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 10060574 9859834 1.02
vortex_q01/duckdb:vortex-file-compressed 5715922 5970599 0.96
duckdb / parquet (1.002x ➖, 0↑ 0↓)
name PR 542e902 (ns) base c42934b (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23301643 23223381 1.00
vortex_q01/duckdb:parquet 9394255 9391854 1.00

No file size changes detected.

@robert3005
less
05ce408 
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
comments
dffff68 
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
nit
542e902 
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 marked this pull request as ready for review July 21, 2026 13:14
@robert3005
robert3005 merged commit 1113b83 into develop Jul 21, 2026
77 checks passed
@robert3005
robert3005 deleted the codex/piecewise-sequence-cursor-copy branch July 21, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes

Assignees

No one assigned

Labels

changelog/performance A performance improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@danking @robert3005 @claude